Feature | Add OpenAPI documentation controller OAuth2UserApiController v2 routes#105
Feature | Add OpenAPI documentation controller OAuth2UserApiController v2 routes#105matiasperrone-exo wants to merge 11 commits intomainfrom
Conversation
e06252a to
bba5793
Compare
caseylocker
left a comment
There was a problem hiding this comment.
- CRITICAL
app/Swagger/Security/UsersOAuth2Schema.php uses hardcoded relative paths:
authorizationUrl: '/oauth2/auth',
tokenUrl: '/oauth2/token',
Per project conventions like in summit-api, these must use the L5-Swagger constants:
authorizationUrl: L5_SWAGGER_CONST_AUTH_URL,
tokenUrl: L5_SWAGGER_CONST_TOKEN_URL,
The .env.example already defines L5_SWAGGER_CONST_AUTH_URL and L5_SWAGGER_CONST_TOKEN_URL, and the config registers them under defaults.constants. Hardcoding defeats the purpose of environment-specific URLs.
app/Http/Controllers/Api/OAuth2/OAuth2UserApiController.php
The getV2 route in routes/api_v2.php is wrapped in service.account middleware:
Route::get('', ['middleware' => 'service.account', 'uses' => 'OAuth2UserApiController@getV2']);
The OpenAPI attribute on getV2 has summary: 'Get a user by ID' but no description mentioning the middleware requirement. Per conventions, it should include something like:
description: 'Requires service account authentication (middleware: service.account)',
UsersOAuth2Schema.php only defines one scope:
IUserScopes::ReadAll => 'Read All Users Data',
The security scheme definition should include all scopes that will be referenced across User endpoints (not just the one used by this PR's endpoint). Looking at IUserScopes, the scheme should also include at minimum: MeRead, Write, UserGroupWrite, and Registration — any scope that future User/Group endpoints will reference.
config/l5-swagger.php Copy/Paste
'title' => 'Summit API Swagger UI',
Should be 'OpenStackID API Swagger UI' or similar. This is a copy-paste from the summit-api project.
396c732 to
9e829a1
Compare
|
Thanks @caseylocker for the comments. The response to each point:
|
|
@caseylocker I checked and added the legend "(only for service accounts)" to the description |
9e829a1 to
b1f6b47
Compare
Signed-off-by: matiasperrone-exo <matias.perrone@exomindset.co>
Signed-off-by: matiasperrone-exo <matias.perrone@exomindset.co>
…els schemas Signed-off-by: matiasperrone-exo <matias.perrone@exomindset.co>
Signed-off-by: matiasperrone-exo <matias.perrone@exomindset.co>
Signed-off-by: matiasperrone-exo <matias.perrone@exomindset.co>
Signed-off-by: matiasperrone-exo <matias.perrone@exomindset.co>
Signed-off-by: matiasperrone-exo <matias.perrone@exomindset.co>
Signed-off-by: matiasperrone-exo <matias.perrone@exomindset.co>
… v2 routes Signed-off-by: matiasperrone-exo <matias.perrone@exomindset.co>
…for URLs Signed-off-by: matiasperrone-exo <matias.perrone@exomindset.co>
Signed-off-by: matiasperrone-exo <matias.perrone@exomindset.co>
b1f6b47 to
429d85e
Compare
Task:
Ref: https://app.clickup.com/t/86b7fj5wh
Endpoints affected: